Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove entries from History/Domain completers when they're removed on chrome. #715

Merged
merged 9 commits into from Nov 13, 2012
Merged

Remove entries from History/Domain completers when they're removed on chrome. #715

merged 9 commits into from Nov 13, 2012

Conversation

smblott-github
Copy link
Collaborator

Currently, if a history entry is removed in chrome (or the entire history is removed), then the effects do not propagate into vimium until the next restart.

This is a pretty inconsistent UX.

The attached code fixes this inconsistency.


(Edit: Comments regarding bug in binarySearch deleted.)

When a chrome history entry is removed, remove that entry from our
history too.  Same when the entire history is removed.
@smblott-github
Copy link
Collaborator Author

Doh! A short walk on the beach and I see how binarySearch is working. I withdraw (and apologize for) the "bug" comments above.

@@ -169,7 +169,7 @@ class HistoryCompleter
# The domain completer is designed to match a single-word query which looks like it is a domain. This supports
# the user experience where they quickly type a partial domain, hit tab -> enter, and expect to arrive there.
class DomainCompleter
domains: null # A map of domain -> history
domains: null # A map of domain -> { entry: <historyEntry>, referenceCount: <count> }
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add a comment here to explain what reference count is and why it exists.

@philc
Copy link
Owner

philc commented Nov 11, 2012

This is a nice case to handle, thanks for tackling it. If you can make the requested changes we'll merge it in.

@smblott-github
Copy link
Collaborator Author

Thanks @philc -- I think all of your issues are addressed now.

@philc
Copy link
Owner

philc commented Nov 13, 2012

lgtm, thanks!

philc added a commit that referenced this pull request Nov 13, 2012
Remove entries from History/Domain completers when they're removed on chrome.
@philc philc merged commit 32e506d into philc:master Nov 13, 2012
davidwallacejackson pushed a commit to davidwallacejackson/vimium that referenced this pull request Apr 3, 2014
davidwallacejackson pushed a commit to davidwallacejackson/vimium that referenced this pull request Apr 3, 2014
Remove entries from History/Domain completers when they're removed on chrome.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants